feat(jest): move Jest config to use a custom react-native Jest env #34971

kelset · 2022-10-13T16:44:23Z

Summary

This PR is the follow up to the conversation started here by @SimenB: react-native-community/discussions-and-proposals#509

Basically, we want to move RN to use its own custom environment so that we can tweak it going forward - this PR in fact only sets up the groundwork for that; @robhogan mentioned that with this in place, Meta engineers can

iterate on it (with jest-environment-node as a starting point) against our internal product tests

This is also connected to Rob's work to bring Jest 29 into the codebase #34724 and my "mirror" PR to bring template in main up to the same version (#34972)

Next steps

This PR opens the door for 3 follow up steps:

actually customize the custom jest env
remove the need of jest-environment-node
publish the env as its own package @react-native/jest-env so that it can be consumed more easily from other tools and developers

Changelog

[General] [Changed] - move Jest config to use a custom react-native Jest env

Test Plan

Tested that yarn test in main works fine after the changes; CI and Meta's internal CI will also serve the purpose of verifying that it works (but there's no reason not to since it's still pretty much just relying on node).

analysis-bot · 2022-10-13T17:06:26Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 8422557
Branch: main

analysis-bot · 2022-10-13T17:14:12Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,776,717	-714
android	hermes	armeabi-v7a	7,177,493	-929
android	hermes	x86	8,089,617	-808
android	hermes	x86_64	8,061,291	-829
android	jsc	arm64-v8a	9,636,692	-189
android	jsc	armeabi-v7a	8,400,781	-387
android	jsc	x86	9,585,781	-277
android	jsc	x86_64	10,178,934	-292

Base commit: 8422557
Branch: main

SimenB

👍

SimenB · 2022-10-13T17:29:59Z

jest/react-native-env.js

+
+'use strict';
+
+const NodeEnv = require('jest-environment-node').TestEnvironment;


Should add a dependency on jest-environment-node - otherwise you rely on hoisting.

When you stop extending the existing env, that dep can of course be removed.

sounds good - originally I added it to the repo-config package.json, but since it didn't change the yarn.lock I assumed it was part of the dependencies that jest itself was bringing along. Will modify PR accordingly 👍

since it didn't change the yarn.lock I assumed it was part of the dependencies that jest itself was bringing along

that's correct, but it would then rely on hoisting 🙂

It needs to be part of the root package.json btw - same level as the preset lives on. Which might introduce issues since there's no jest there... Possibly an optional peer dep makes more sense than a direct dependency? Again, will be removed when you stop extending.

(it might make sense to just copy the entire content of jest-environment-node now instead of empty extend? That way, no dep)

It needs to be part of the root package.json btw - same level as the preset lives on. Which might introduce issues since there's no jest there... Possibly an optional peer dep makes more sense than a direct dependency? Again, will be removed when you stop extending.

yeah the thing is that the repo-config/package.json is this really weird thing (the plan is to remove it)... goes back to the monorepo conversation here react-native-community/discussions-and-proposals#480

I'm going to guess this dep should live alongside Jest, and that means that I'll have to add it to the repo-config one and then modify the magic script that does some stuff when branch cutting to ensure that on released versions it gets moved along (you can see what I mean here c012726#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519)

That won't hit same issue as #31668 (comment)?

Very true 😅 separate package for the preset entirely is probably the least wrong approach (longer term - short term this should work a treat of course)

right, almost forgot - added a "next steps" section to the PR body with all the things that should be done as follow ups :)

I feel like I mentioned it but I can't find it so might've been a fever dream.

An option to a dep is optional peer dep. Sorta depends on timeline for 0.71 - seems suboptimal for yarn add react-native to bundle a jest env.

An option to a dep is optional peer dep. Sorta depends on timeline for 0.71 - seems suboptimal for yarn add react-native to bundle a jest env.

Fair point. Correct me if I'm wrong - that would also mean adding jest-environment-node to the template's devDependencies explicitly (alongside jest)? I don't think peer dependencies are guaranteed to be hoisted if they're only provided transitively (via jest in this case), I think they're expected to be specified by the root project.

We'd have to introduce peerDependenciesMeta also, since react-native doesn't currently have any optional peer deps.

I'm ok with the current dependencies approach for a few reasons;

simplicity, especially re upgrade paths for userland package.json,

jest-environment-node is small,

we're moving towards a monorepo,

we plan to drop the dependency on jest-environment-node

@cortinico, @hramos , @motiz88 - want to weigh in here?

(for context: we are aiming to cut 0.71 over the next couple weeks)

github-actions · 2022-10-14T10:15:09Z

	Warnings
⚠️	🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS against 7026c14

facebook-github-bot · 2022-10-14T10:21:10Z

@robhogan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

SimenB · 2022-10-17T15:58:35Z

jest/react-native-env.js

+const NodeEnv = require('jest-environment-node').TestEnvironment;
+
+module.exports = class ReactNativeEnv extends NodeEnv {
+  // this is where we'll add our custom logic


I see an RFC for exports is up now, with the expected react-native condition: https://github.com/react-native-community/discussions-and-proposals/pull/534/files#diff-40781647168e437e711b426935003e99ce4bddb4c1a5f1ebd61b0218d2b45670R294

Might make sense to override exportConditions right away? Or do you want this PR to be 100% compatible, then make changes to the env iteratively?

I guess ideally this env would get the option from metro/metro config somehow instead of defining its own

100% compatible, then make changes to the env iteratively?

yeah I'd rather have this one be compatible, then do things on top. Maybe post a comment in the RFC itself btw?

which of them? the one I opened about the env or the one for exports?

the export one, it will be a fairly important one for a couple reasons so if we have a reminder of this topic in there it's more likely we'll keep it in mind

cool, added some comments now 👍

react-native-bot · 2022-10-17T19:45:05Z

This pull request was successfully merged by @kelset in cb2dcd3.

^{When will my fix make it into a release? | Upcoming Releases}

Summary: This PR is the follow up of #34724 for the template; this way, we'll have version alignment and RN71 will use this. This wants to be merged along #34971 ## Changelog  [General] [Changed] - upgrade Jest in template to 29 Pull Request resolved: #34972 Test Plan: Generated a test project via `yarn test-e2e-local -t RNTestProject` and then run `yarn test` in that project to verify that there's no regression. ✅ Reviewed By: huntie Differential Revision: D40379963 Pulled By: robhogan fbshipit-source-id: 618207ed6bf7921d13f0b6a9220dc52c9cf78b14

…acebook#34971) Summary: This PR is the follow up to the conversation started here by SimenB: react-native-community/discussions-and-proposals#509 Basically, we want to move RN to use its own custom environment so that we can tweak it going forward - this PR in fact only sets up the groundwork for that; robhogan mentioned that with this in place, Meta engineers can > iterate on it (with jest-environment-node as a starting point) against our internal product tests This is also connected to Rob's work to bring Jest 29 into the codebase facebook#34724 and my "mirror" PR to bring template in main up to the same version (facebook#34972) ## Changelog  [General] [Changed] - move Jest config to use a custom react-native Jest env Pull Request resolved: facebook#34971 Test Plan: Tested that `yarn test` in main works fine after the changes; CI and Meta's internal CI will also serve the purpose of verifying that it works (but there's no reason not to since it's still pretty much just relying on `node`). Reviewed By: huntie Differential Revision: D40379760 Pulled By: robhogan fbshipit-source-id: 2c6d0bc86d337fda9befce0799bda2f56cc4466c

Summary: This PR is the follow up of facebook#34724 for the template; this way, we'll have version alignment and RN71 will use this. This wants to be merged along facebook#34971 ## Changelog  [General] [Changed] - upgrade Jest in template to 29 Pull Request resolved: facebook#34972 Test Plan: Generated a test project via `yarn test-e2e-local -t RNTestProject` and then run `yarn test` in that project to verify that there's no regression. ✅ Reviewed By: huntie Differential Revision: D40379963 Pulled By: robhogan fbshipit-source-id: 618207ed6bf7921d13f0b6a9220dc52c9cf78b14

add a custom react-native jest env

de24efa

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Microsoft Partner: Microsoft Partner labels Oct 13, 2022

kelset mentioned this pull request Oct 13, 2022

feat(jest): bump jest to 29 in the template #34972

Closed

prettier

399651a

kelset requested a review from robhogan October 13, 2022 16:49

SimenB reviewed Oct 13, 2022

View reviewed changes

adding to the deps in root package.json

7026c14

kelset requested a review from hramos as a code owner October 14, 2022 10:13

robhogan approved these changes Oct 14, 2022

View reviewed changes

SimenB reviewed Oct 17, 2022

View reviewed changes

facebook-github-bot closed this in cb2dcd3 Oct 17, 2022

react-native-bot added the Merged This PR has been merged. label Oct 17, 2022

kelset deleted the kelset/add-jest-custom-env branch October 18, 2022 09:17

SimenB mentioned this pull request Oct 18, 2022

Create a custom Jest environment react-native-community/discussions-and-proposals#509

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(jest): move Jest config to use a custom react-native Jest env #34971

feat(jest): move Jest config to use a custom react-native Jest env #34971

kelset commented Oct 13, 2022 •

edited

Loading

analysis-bot commented Oct 13, 2022 •

edited

Loading

analysis-bot commented Oct 13, 2022 •

edited

Loading

SimenB left a comment

SimenB Oct 13, 2022

kelset Oct 14, 2022

SimenB Oct 14, 2022 •

edited

Loading

kelset Oct 14, 2022

SimenB Oct 14, 2022 •

edited

Loading

SimenB Oct 14, 2022

kelset Oct 14, 2022

SimenB Oct 14, 2022

robhogan Oct 14, 2022 •

edited

Loading

kelset Oct 14, 2022

github-actions bot commented Oct 14, 2022

facebook-github-bot commented Oct 14, 2022

SimenB Oct 17, 2022

SimenB Oct 17, 2022

kelset Oct 18, 2022

SimenB Oct 18, 2022

kelset Oct 18, 2022

SimenB Oct 18, 2022

react-native-bot commented Oct 17, 2022


		'use strict';

		const NodeEnv = require('jest-environment-node').TestEnvironment;

feat(jest): move Jest config to use a custom react-native Jest env #34971

feat(jest): move Jest config to use a custom react-native Jest env #34971

Conversation

kelset commented Oct 13, 2022 • edited Loading

Summary

Next steps

Changelog

Test Plan

analysis-bot commented Oct 13, 2022 • edited Loading

analysis-bot commented Oct 13, 2022 • edited Loading

SimenB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robhogan Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 14, 2022

facebook-github-bot commented Oct 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

react-native-bot commented Oct 17, 2022

kelset commented Oct 13, 2022 •

edited

Loading

analysis-bot commented Oct 13, 2022 •

edited

Loading

analysis-bot commented Oct 13, 2022 •

edited

Loading

SimenB Oct 14, 2022 •

edited

Loading

SimenB Oct 14, 2022 •

edited

Loading

robhogan Oct 14, 2022 •

edited

Loading